-
-
Notifications
You must be signed in to change notification settings - Fork 359
[iOS only] Add the ability to intercept errors from native side and forward them to JS console #5622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- An (ongoing) experiment with adding native logs listener ([#5622](https://github.com/getsentry/sentry-react-native/pull/5622))If none of the above apply, you can opt out of this check by adding |
2108fe1 to
f369c1d
Compare
| if (Platform.OS !== 'ios' && Platform.OS !== 'android') { | ||
| debug.log('Native log listener is only supported on iOS and Android.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work on the future once we have support for Android and iOS. For the time being, this PR is only adding support for iOS
| if (Platform.OS !== 'ios' && Platform.OS !== 'android') { | |
| debug.log('Native log listener is only supported on iOS and Android.'); | |
| if (Platform.OS !== 'ios') { | |
| debug.log('Native log listener is only supported on iOS.''); |
| const prefix = `[Sentry] [${log.level.toUpperCase()}] [${log.component}]`; | ||
| const message = `${prefix} ${log.message}`; | ||
|
|
||
| switch (log.level.toLowerCase()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if defaultNativeLogHandler doesn't create any events when captureConsoleIntegration is added as an integration.
Q: Shouldn't we use debug instead of console here?
lucas-zimerman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far it's looking good! I left a few suggestions that would be nice to be addressed before merging.
📢 Type of change
📜 Description
Was born as an attempt to fix #5566 and turned out to be a good idea in general.
This PR makes it possible to intercept native logs (currently only works with iOS and only with a patch, see notes below) and forward them to JS console:
(that only works when
debugis set totrue)This PR makes it so that instead of handling HTTP code 413 somehow separately we can allow users to just get native logs in their JS console and see code 413 or any other error codes themselves. Filtering per level or component is also possible.
Notes
Here is the patch (save it to
setOutput.patchand apply withgit apply setOutput.patchbefore building the app):📝 Checklist
sendDefaultPIIis enabled🔮 Next steps